Skip to content

#28,29 - 유저는 투표 결과를 조회할 수 있다. #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Jul 23, 2024

Conversation

kpeel5839
Copy link
Contributor

@kpeel5839 kpeel5839 commented Jul 22, 2024

1. 🔗 관련 이슈

#28
#29

2. 📄 구현한 내용 또는 수정한 내용

1~5등, 1등 전체조회하는 코드 작성했어요!

3. 🙌 추가적으로 알리고 싶은 내용

추가적으로 submodule 가져올 수 있도록 yml 변경했어요.

그리고 코드가 굉장히 긴 것처럼 나오는데, 거의 다 테스트이니 걱정은 노노햇!

4. 🙄 TODO / 고민하고 있는 것들

인수테스트를 작성하려 했는데, Token 값을 어떻게 넘겨야하는지 여쭤봐야겠더라고요! 이번에 알려주시면 다음에 Controller Test까지 작성하겠습니다.

5. ✅ 배포 Checklist

  • 본인을 Assign 해주세요.
  • 본인을 제외한 백엔드 개발자를 리뷰어로 지정해주세요.

@kpeel5839 kpeel5839 requested a review from sectionr0 July 22, 2024 12:51
@kpeel5839 kpeel5839 self-assigned this Jul 22, 2024
@kpeel5839 kpeel5839 added 🌱기능🌱 새로운 기능을 추가해요 ! 🔥리뷰 기다리는 중🔥 PR을 올리고 코드리뷰를 기다리고 있는 상태입니다. 🏋️매튜🏋️ 24기 김재연 labels Jul 22, 2024
Copy link
Contributor

@sectionr0 sectionr0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 코드 깔끔하게 잘 짜셨네요👍👍 고생하셨습니다!

rankedResults[2].voteCount shouldBe 1
}
}
val calculateRankByBallotSizeAndReceivedAt = listOf(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

변수명 엄청 기네요😭

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅋㅋㅋㅋㅋㅋㅋㅋ 어쩌다보니.. 테스트니까 이해해주세요오옹 ♡♡

import com.wespot.voteoption.VoteOption
import java.time.LocalDate

object VoteServiceHelper {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헬퍼를 따로 만들어서 한 번에 모아뒀군요! 👍👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

생각보다 안겹치더라고요! 모아놓아도 문제 없을 것 같아 헬퍼로 구분했어요

@sectionr0
Copy link
Contributor

인수 테스트 관련한 거는 현재 토큰을 받을 수 있는 방식이 소셜로그인 접근을 해서 받아야 하는데,, 그 과정은 너무 복잡할 것 같아요🥲

따로 어드민용 로그인 API (단순히 이메일, 패스워드 넣어서 토큰 받을 수 있는 API) 만들어서 그걸로 간단하게 진행하면 좋을 것 같은데,,,

재연님 생각은 어떠신가요?!

@kpeel5839
Copy link
Contributor Author

인수 테스트 관련한 거는 현재 토큰을 받을 수 있는 방식이 소셜로그인 접근을 해서 받아야 하는데,, 그 과정은 너무 복잡할 것 같아요🥲

따로 어드민용 로그인 API (단순히 이메일, 패스워드 넣어서 토큰 받을 수 있는 API) 만들어서 그걸로 간단하게 진행하면 좋을 것 같은데,,,

재연님 생각은 어떠신가요?!

테스트 코드를 위한 코드는 별로 좋아하지 않는 편이긴한데, 지금은 말씀하신 것처럼 어쩔 수 없을 것 같아요!

동의합니다 흐흐

@kpeel5839 kpeel5839 merged commit 83f6bca into develop Jul 23, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱기능🌱 새로운 기능을 추가해요 ! 🏋️매튜🏋️ 24기 김재연 🔥리뷰 기다리는 중🔥 PR을 올리고 코드리뷰를 기다리고 있는 상태입니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants